Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix note and receptor scaling and note tail offsets #3351

Merged
merged 2 commits into from
Sep 30, 2024
Merged

Fix note and receptor scaling and note tail offsets #3351

merged 2 commits into from
Sep 30, 2024

Conversation

dombomb64
Copy link
Contributor

@dombomb64 dombomb64 commented Sep 17, 2024 •
edited
Loading

Does this PR close any issues? If so, link them below.

#3323 (Actually wait only some of the things mentioned there are fixed, only the third bullet and maybe the second one, if you count my duct-tape fix of putting offsets for the hold notes to counteract the offset that I couldn’t find the cause of as an actual fix)

Briefly describe the issue(s) fixed.

  • Made note asset scaling easier to understand. Before, there was some math going on behind the scenes (the hardcoded scaling stuff was probably added before the JSON scaling was implemented), but now, it just uses the scale in the JSON as the scale to apply to the sprites.
  • Added offset functionality to the note tail sprites. The attached JSON for the pixel art notes uses this to pretty much fix the tails' offsets.
  • In turn, I was able to make the pixel art notes have a scale factor that doesn't have mixels. The tails' scale has also been updated to keep a consistent pixel size.

Include any relevant screenshots or videos.

Note tails before adding offset functionality (but after fixing receptor and note scaling)
Note tails before adding offset functionality (but after fixing receptor and note scaling)

NOTE:

If this pull request is accepted, these are the new JSON files for the note styles.
funkin.json
pixel.json

@dombomb64
Copy link
Contributor Author

Forgot to mention that most of this code was written on the main branch and then transferred over to the correct one because I couldn't figure out how to get the develop branch to compile properly.

@dombomb64 dombomb64 changed the title Fix note sprite stuff Fix note and receptor scaling and note tail offsets Sep 17, 2024
@EliteMasterEric EliteMasterEric added the status: pending triage The bug or PR has not been reviewed yet. label Sep 18, 2024
@EliteMasterEric
Copy link
Member

EliteMasterEric commented Sep 18, 2024 •
edited
Loading

Do you have a good screenshot of how the hold note looks after your changes?

EDIT: Got one after way too many attempts:
crop

@EliteMasterEric EliteMasterEric added type: enhancement Provides an enhancement or new feature. status: needs clarification Requires more info from the contributor. labels Sep 18, 2024
@EliteMasterEric EliteMasterEric self-assigned this Sep 18, 2024
@EliteMasterEric EliteMasterEric added status: reviewing internally This PR is under internal review and quality assurance testing and removed status: pending triage The bug or PR has not been reviewed yet. status: needs clarification Requires more info from the contributor. labels Sep 18, 2024
@EliteMasterEric EliteMasterEric added this to the 0.5.1 milestone Sep 19, 2024
@EliteMasterEric EliteMasterEric added status: accepted Approved for contribution. If it's not already merged, it may be merged on a private branch. and removed status: reviewing internally This PR is under internal review and quality assurance testing labels Sep 19, 2024
@dombomb64
Copy link
Contributor Author

I realize now that most of the stuff mentioned in the issue I linked isn’t fixed by this pull request except for the scale of the note tails, but I still think it should be merged because it fixes other things. Also, on the topic of the note tails, I’m starting to think that there should be a better fix to the offsets rather than just counteracting them in the json (although I do still want the functionality for the json offsets kept), but I can’t begin to wrap my head around how the rendering code for them works right now, so I don’t really know what’s causing them to be slightly offset.

@AbnormalPoof AbnormalPoof mentioned this pull request Sep 26, 2024
Closed
@ninjamuffin99 ninjamuffin99 merged commit 6e33cac into FunkinCrew:develop Sep 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@EliteMasterEric EliteMasterEric

Labels
status: accepted Approved for contribution. If it's not already merged, it may be merged on a private branch. type: enhancement Provides an enhancement or new feature.
Projects
None yet
Milestone
0.5.1
Development

Successfully merging this pull request may close these issues.
3 participants
@dombomb64 @EliteMasterEric @ninjamuffin99